Bugfix Thread instrumentation continuing to use finished transactions on other threads #1969
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
finished transaction
used in new threads
tannalynn
requested review from
fallwith,
hannahramadan and
kaylareopelle
as code owners
SimpleCov Report
|
fallwith
approved these changes
Apr 25, 2023
| @@ -29,6 +29,12 @@ def add_segment(segment, parent = nil) | |||
| else | |||
| segment.record_on_finish = true | |||
| ::NewRelic::Agent.logger.debug("Segment limit of #{segment_limit} reached, ceasing collection.") | |||
|
|
|||
| if finished? | |||
| ::NewRelic::Agent.logger.debug("Transaction #{best_name} has finished but segments still being created, resetting state.") | |||
There was a problem hiding this comment.
This new module code make use of a method, best_name, that doesn't exist in the module, so the code can only be used by an instance of the Transaction class. Seeing as the Transaction class is the only consumer (includer) of this Tracing module, this is probably ok.
hannahramadan
approved these changes
Apr 26, 2023
kaylareopelle
added a commit
that referenced
this pull request
May 1, 2023
The NewRelic::Agent::PipeChannelManagerTest has two cases failing on Ruby 2.5.9 and Rails 6.0/6.1: #test_listener_merges_error_events, #test_listener_merges_error_traces These tests started failing after PR#1969 was merged: #1969 The tests are now skipped in the specific conditions where they intermittently fail
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a bug where the agent would continue to record segments on transactions that were already finished in another thread. This would lead to increased memory usage over time because the agent would continue to copy over the finished transaction as the still current transaction in the thread instrumentation, causing the agent to think it was still supposed to continue recording data on the actually finished transaction. Now the agent will check the finished state of a transaction before using it when a new thread is spawned.
Additionally, if a transaction reaches the segment limit but the transaction is actually already finished on some other thread, the state will now reset, preventing future segments from being created and a supportability metric will be recorded.
#1936